Skip to content
This repository has been archived by the owner on Feb 1, 2022. It is now read-only.

Allow multiple filter flags in Table #141

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arnulfojr
Copy link

Sorry for creating this PR out of the blue.

I wanted to be able to filter the table rows applying multiple filters.
Let's take this PR as a discussion starter:)

@salesforce-cla
Copy link

Thanks for the contribution! Unfortunately we can't verify the commit author(s): Arnulfo Solis <a***@A***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, sign the Salesforce.com Contributor License Agreement and this Pull Request will be revalidated.

@salesforce-cla
Copy link

Thanks for the contribution! Before we can merge this, we need @arnulfojr to sign the Salesforce.com Contributor License Agreement.

@codecov
Copy link

codecov bot commented Feb 26, 2020

Codecov Report

Merging #141 into master will decrease coverage by 60.88%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #141       +/-   ##
==========================================
- Coverage   60.88%   0.00%   -60.89%     
==========================================
  Files          15       9        -6     
  Lines         519     165      -354     
  Branches      110      37       -73     
==========================================
- Hits          316       0      -316     
+ Misses        180     165       -15     
+ Partials       23       0       -23     
Impacted Files Coverage Δ
src/styled/progress.ts 0.00% <0.00%> (-87.50%) ⬇️
src/prompt.ts 0.00% <0.00%> (-84.71%) ⬇️
src/styled/tree.ts 0.00% <0.00%> (-75.00%) ⬇️
src/styled/object.ts 0.00% <0.00%> (-70.38%) ⬇️
src/index.ts 0.00% <0.00%> (-56.17%) ⬇️
src/action/base.ts
src/exit.ts
src/action/simple.ts
src/deps.ts
src/config.ts

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33089da...8fec520. Read the comment docs.

@dotnetCarpenter
Copy link

@arnulfojr I don't get it. I can add multiple filters with version 5.4.5. Github says "This branch has no conflicts with the base branch", so I wonder, what did you exactly change?

E.g.

./bin/run read -t payments --filter=charged=false --filter=firefundpercentage=100 -x

rows where charged = false and firefundpercentage = 100

I pass the flags unaltered to cli.table()...

cli.table(dataList, convertToTableColumns(dataList), flags)

@MunifTanjim
Copy link
Contributor

I don't get it. I can add multiple filters with version 5.4.5.

You can pass that. But they are not processed.

@dotnetCarpenter
Copy link

@MunifTanjim in my case they are processed and the table only renders the data that match my filters.

If I get time I can create a reduced test case but I suspect that you have experienced an edge case, that I am not seeing.

@MunifTanjim
Copy link
Contributor

If I get time I can create a reduced test case but I suspect that you have experienced an edge case, that I am not seeing.

Data

x=1   y=a1
x=1   y=a2
x=1   y=b1
x=2   y=b2
x=2   y=c1

--filter=x=1 --filter=y=b

Expected Output

x=1   y=b1

Actual Output

Depending on the order of the passed filters,

x=1   y=a1
x=1   y=a2
x=1   y=b1

or

x=1   y=b1
x=2   y=b2

@RasPhilCo
Copy link
Contributor

RasPhilCo commented Jun 16, 2020

Cool! Can you add a multiple filter test? If you'd rather pass to #156 that's fine too.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants